Skip to content

fix the doc-comment-decoration-trimming edge-case rustdoc ICE #47210

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Conversation

zackmdavis
Copy link
Member

This horizontal_trim function strips the leading whitespace from
doc-comments that have a left-asterisk-margin:

  /**
   * You know what I mean—
   *
   * comments like this!
   */

The index of the column of asterisks is i, and if trimming is deemed
possible, we slice each line from i+1 to the end of the line. But if, in
particular, i was 0 and there was an empty line (as in the example
given in the reporting issue), we ended up panicking trying to slice an
empty string from 0+1 (== 1).

Let's tighten our check to say that we can't trim when i is even the same
as the length of the line, not just when it's greater. (Any such cases
would panic trying to slice line from line.len()+1.)

Resolves #47197.

@rust-highfive
Copy link
Contributor

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@zackmdavis
Copy link
Member Author

r? @QuietMisdreavus

@QuietMisdreavus
Copy link
Member

Can you add a test for this? Something like this:

src/test/rustdoc/issue-47197.rs:

// copyright header

// libsyntax ICE when trimming the blank line in the middle of that comment

// @has issue_47197/fn.x.html
/**
* a

* b
*/
pub fn x() { }

@shepmaster shepmaster added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 6, 2018
This `horizontal_trim` function strips the leading whitespace from
doc-comments that have a left-asterisk-margin:

  /**
   * You know what I mean—
   *
   * comments like this!
   */

The index of the column of asterisks is `i`, and if trimming is deemed
possible, we slice each line from `i+1` to the end of the line. But if, in
particular, `i` was 0 _and_ there was an empty line (as in the example
given in the reporting issue), we ended up panicking trying to slice an
empty string from 0+1 (== 1).

Let's tighten our check to say that we can't trim when `i` is even the same
as the length of the line, not just when it's greater. (Any such cases
would panic trying to slice `line` from `line.len()+1`.)

Resolves rust-lang#47197.
@zackmdavis zackmdavis force-pushed the the_3rd_of_2_hardest_problems_in_computer_science branch from b6bde34 to 3cfea33 Compare January 6, 2018 19:18
@zackmdavis
Copy link
Member Author

(updated)

@QuietMisdreavus
Copy link
Member

@bors r+ rollup

Thanks!

@bors
Copy link
Collaborator

bors commented Jan 6, 2018

📌 Commit 3cfea33 has been approved by QuietMisdreavus

@kennytm kennytm added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 8, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Jan 8, 2018
…blems_in_computer_science, r=QuietMisdreavus

fix the doc-comment-decoration-trimming edge-case rustdoc ICE

This `horizontal_trim` function strips the leading whitespace from
doc-comments that have a left-asterisk-margin:

```
  /**
   * You know what I mean—
   *
   * comments like this!
   */
```

The index of the column of asterisks is `i`, and if trimming is deemed
possible, we slice each line from `i+1` to the end of the line. But if, in
particular, `i` was 0 _and_ there was an empty line (as in the example
given in the reporting issue), we ended up panicking trying to slice an
empty string from 0+1 (== 1).

Let's tighten our check to say that we can't trim when `i` is even the same
as the length of the line, not just when it's greater. (Any such cases
would panic trying to slice `line` from `line.len()+1`.)

Resolves rust-lang#47197.
bors added a commit that referenced this pull request Jan 9, 2018
Rollup of 10 pull requests

- Successful merges: #47210, #47233, #47246, #47254, #47256, #47258, #47259, #47263, #47270, #47272
- Failed merges: #47248
@bors bors merged commit 3cfea33 into rust-lang:master Jan 9, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants